Skip to content

Conversation

@dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Nov 2, 2024

This patch intersects poison-generating flags after CSE to fix assertion failure reported in #112354 (comment).
It is an alternative to #114582.

Co-authored-by: Antonio Frighetto [email protected]

@llvmbot
Copy link
Member

llvmbot commented Nov 2, 2024

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-arm

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch intersects poison-generating flags after CSE to fix assertion failure reported in #112354 (comment).
It is an alternative to #114582.

Co-authored-by: Antonio Frighetto <[email protected]>


Full diff: https://github.com/llvm/llvm-project/pull/114650.diff

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+6-2)
  • (added) llvm/test/CodeGen/ARM/dagcombine-drop-flags-freeze.ll (+23)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index d5cdd7163d7910..166b6dbf46db87 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -10318,8 +10318,10 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
     AddNodeIDNode(ID, Opcode, VTs, Ops);
     void *IP = nullptr;
 
-    if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP))
+    if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP)) {
+      E->intersectFlagsWith(Flags);
       return SDValue(E, 0);
+    }
 
     N = newSDNode<SDNode>(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs);
     createOperands(N, Ops);
@@ -10524,8 +10526,10 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, SDVTList VTList,
     FoldingSetNodeID ID;
     AddNodeIDNode(ID, Opcode, VTList, Ops);
     void *IP = nullptr;
-    if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP))
+    if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP)) {
+      E->intersectFlagsWith(Flags);
       return SDValue(E, 0);
+    }
 
     N = newSDNode<SDNode>(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTList);
     createOperands(N, Ops);
diff --git a/llvm/test/CodeGen/ARM/dagcombine-drop-flags-freeze.ll b/llvm/test/CodeGen/ARM/dagcombine-drop-flags-freeze.ll
new file mode 100644
index 00000000000000..63e16b03caee6b
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/dagcombine-drop-flags-freeze.ll
@@ -0,0 +1,23 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=arm-eabi -o - %s | FileCheck %s
+
+; Ensure poison-generating flags are stripped by the time a freeze operand is visited.
+
+define i1 @drop_flags(i32 noundef %numentries, i64 %cond, i64 %arg) {
+; CHECK-LABEL: drop_flags:
+; CHECK:       @ %bb.0: @ %entry
+; CHECK-NEXT:    ldm sp, {r1, r12}
+; CHECK-NEXT:    subs r1, r2, r1
+; CHECK-NEXT:    sbcs r1, r3, r12
+; CHECK-NEXT:    movlo r0, r2
+; CHECK-NEXT:    rsbs r1, r0, #0
+; CHECK-NEXT:    adc r0, r0, r1
+; CHECK-NEXT:    mov pc, lr
+entry:
+  %cmp4 = icmp samesign ult i64 %cond, %arg
+  %conv6 = trunc nuw i64 %cond to i32
+  %spec.select = select i1 %cmp4, i32 %conv6, i32 %numentries
+  %spec.select.fr = freeze i32 %spec.select
+  %cmpz = icmp eq i32 %spec.select.fr, 0
+  ret i1 %cmpz
+}

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Flags were already intersected in the methods that take a fixed number of operands, but the generic methods were missed.

@dtcxzyw dtcxzyw merged commit 917b3d1 into llvm:main Nov 2, 2024
11 checks passed
@dtcxzyw dtcxzyw deleted the fix-sdag-drop-flags branch November 2, 2024 11:06
@dtcxzyw
Copy link
Member Author

dtcxzyw commented Nov 2, 2024

@antoniofrighetto You can reland #112354 now.

@antoniofrighetto
Copy link
Contributor

@antoniofrighetto You can reland #112354 now.

Thanks for fixing this.

smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this pull request Nov 3, 2024
This patch intersects poison-generating flags after CSE to fix assertion
failure reported in
llvm#112354 (comment).

Co-authored-by: Antonio Frighetto <[email protected]>
@RKSimon
Copy link
Collaborator

RKSimon commented Nov 3, 2024

Thanks @antoniofrighetto and @dtcxzyw

NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
This patch intersects poison-generating flags after CSE to fix assertion
failure reported in
llvm#112354 (comment).

Co-authored-by: Antonio Frighetto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:ARM llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants